Add pump battery to info display and alarm options #496
Conversation
mjliedtke
commented
Jan 2, 2026
mjliedtke
commented
commit 0d8ae69 Author: Claude <[email protected]> Date: Fri Jan 2 19:59:18 2026 +0000 Add pumpBattery case to AlarmSelectionView switch commit b67fb8b Author: Claude <[email protected]> Date: Fri Jan 2 19:55:21 2026 +0000 Add pumpBattery case to AlarmEditor switch statement Reuses BatteryAlarmEditor for pump battery alarm configuration since both use threshold-based percentage settings. commit 89cc3da Author: Claude <[email protected]> Date: Fri Jan 2 19:46:56 2026 +0000 Add pump battery display and alarm feature - Add pumpBattery to InfoType enum for Information Display table - Parse pump battery percentage from Nightscout API (pump.battery.percent) - Add pumpBatteryLevel observable for alarm data - Add pumpBattery alarm type with threshold-based triggering - Create PumpBatteryCondition for alarm evaluation - Register pump battery alarm in AlarmManager
|
Thanks for the new PR — it looks good overall. For the sound, I think it’s reasonable to reuse the same one (soundFile = .machineCharge). Reusing the same editor is a clever trick, but I think it deserves a short comment, so anyone changing BatteryAlarmEditor in the future understands that it affects both alarms. |
|
Great. Thanks for the feedback and that all makes sense. |
|
@bjorkert I'm actually now wondering if would be better to split the Editor so that the info text makes sense. |
|
Sounds good! |
- Change pump battery icon from battery.25 to powermeter - Split BatteryAlarmEditor into PhoneBatteryAlarmEditor and PumpBatteryAlarmEditor - Each editor now has specific text for its respective battery type
* Refactor battery alarm editors and update pump battery icon - Change pump battery icon from battery.25 to powermeter - Split BatteryAlarmEditor into PhoneBatteryAlarmEditor and PumpBatteryAlarmEditor - Each editor now has specific text for its respective battery type
marionbarker
left a comment
marionbarker
left a comment
There was a problem hiding this comment.
Approve from code review and test except for the inconsistency noted in the code.
Please either change the words, or change the logic for when the alarm is true.
I'm ready to approve once this is fixed.
| guard let limit = alarm.threshold, limit > 0 else { return false } | ||
| guard let level = data.latestPumpBattery else { return false } | ||
|
|
||
| return level <= limit |
There was a problem hiding this comment.
The words say Battery Below, but this line is true for less than or equal to.
It was convenient for testing (because both my MDT pumps are reporting 100%), but this should be consistent.
3 participants
